Skip to content

Conversation

@rniwa
Copy link
Contributor

@rniwa rniwa commented Feb 6, 2025

We should skip the first argument for CXXOperatorCallExpr, not other kinds of calls.

…ape]] on a member function no longer works.

We should skip the first argument for CXXOperatorCallExpr, not other kinds of calls.
@rniwa rniwa requested a review from t-rasmud February 6, 2025 06:55
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Feb 6, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 6, 2025

@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: Ryosuke Niwa (rniwa)

Changes

We should skip the first argument for CXXOperatorCallExpr, not other kinds of calls.


Full diff: https://github.com/llvm/llvm-project/pull/126016.diff

2 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp (+1-3)
  • (modified) clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp (+23)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
index 53ef423bd82e7e2..a56f48c83c660a9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
@@ -109,9 +109,7 @@ class UncountedLambdaCapturesChecker
       bool VisitCallExpr(CallExpr *CE) override {
         checkCalleeLambda(CE);
         if (auto *Callee = CE->getDirectCallee()) {
-          unsigned ArgIndex = 0;
-          if (auto *CXXCallee = dyn_cast<CXXMethodDecl>(Callee))
-            ArgIndex = CXXCallee->isInstance();
+          unsigned ArgIndex = isa<CXXOperatorCallExpr>(CE);
           bool TreatAllArgsAsNoEscape = shouldTreatAllArgAsNoEscape(Callee);
           for (auto *Param : Callee->parameters()) {
             if (ArgIndex >= CE->getNumArgs())
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
index 2a1a164557cdbe7..b6e84769fdf7247 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
@@ -63,6 +63,18 @@ template<typename Out, typename... In> Function<Out(In...)> adopt(Detail::Callab
     return Function<Out(In...)>(impl, Function<Out(In...)>::Adopt);
 }
 
+template <typename KeyType, typename ValueType>
+class HashMap {
+public:
+  HashMap();
+  HashMap([[clang::noescape]] const Function<ValueType()>&);
+  void ensure(const KeyType&, [[clang::noescape]] const Function<ValueType()>&);
+  bool operator+([[clang::noescape]] const Function<ValueType()>&) const;
+
+private:
+  ValueType* m_table { nullptr };
+};
+
 } // namespace WTF
 
 struct A {
@@ -268,6 +280,17 @@ struct RefCountableWithLambdaCapturingThis {
       nonTrivial();
     });
   }
+
+  void method_captures_this_in_template_method() {
+    RefCountable* obj = make_obj();
+    WTF::HashMap<int, RefPtr<RefCountable>> nextMap;
+    nextMap.ensure(3, [&] {
+      return obj->next();
+    });
+    nextMap+[&] {
+      return obj->next();
+    };
+  }
 };
 
 struct NonRefCountableWithLambdaCapturingThis {

class HashMap {
public:
HashMap();
HashMap([[clang::noescape]] const Function<ValueType()>&);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also have a similar test case for a non-member function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good point! Added a test case for static function & non-member function.

Copy link
Contributor

@t-rasmud t-rasmud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@rniwa rniwa merged commit 6f50849 into llvm:main Feb 7, 2025
8 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Feb 7, 2025

LLVM Buildbot has detected a new failure on builder clang-cmake-x86_64-avx512-win running on avx512-intel64-win while building clang at step 4 "cmake stage 1".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/81/builds/4482

Here is the relevant piece of the build log for the reference
Step 4 (cmake stage 1) failure: 'cmake -G ...' (failure)
'cmake' is not recognized as an internal or external command,
operable program or batch file.

Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
rniwa added a commit to rniwa/llvm-project that referenced this pull request Feb 13, 2025
@rniwa rniwa deleted the fix-lambda-noescape-regression branch February 15, 2025 09:40
devincoughlin pushed a commit to swiftlang/llvm-project that referenced this pull request Feb 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:static analyzer clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants